Skip to content

Order creation: Prevent subscription products to be added to an order #15960

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
merged 8 commits into from
Aug 4, 2025
Merged

Order creation: Prevent subscription products to be added to an order #15960

merged 8 commits into from
Aug 4, 2025

Conversation

@iamgabrielma
Copy link
Contributor

@iamgabrielma iamgabrielma commented Jul 30, 2025 •
edited
Loading

Closes WOOMOB-938

Description

This PR disallows adding product of type subscription and variableSubscription to an order via creation or edition flows. Since subscriptions are not currently supported in the app, restricting adding these product types from the get-go will set the expectations right away and remove potential confusion and issues like the one raised in WOOMOB-938

Changes

We leverage the existing bookings implementation and add these two product types as well to the mix. This will disallow the specific product type from being selected and added to the order in both creation and edition flows.

Testing information

  • Install WooCommerce Subscriptions plugin
  • Create a subscription product either in wp-admin or via the app. Note that subscription products can still be created via the Product tab, this decision is outside of the scope of this PR
  • Navigate to the Orders tab, attempt to create or edit an order adding a subscription product, and confirm that is not possible.

Screenshots

Screen.Recording.2025-07-30.at.15.14.25.mov
  • I have considered if this change warrants user-facing release notes and have added them to RELEASE-NOTES.txt if necessary.

@wpmobilebot
Copy link
Collaborator

wpmobilebot commented Jul 30, 2025 •
edited
Loading

App Icon📲 You can test the changes from this Pull Request in WooCommerce iOS Prototype by scanning the QR code below to install the corresponding build.

App NameWooCommerce iOS Prototype
Build Numberpr15960-04fab3b
Version22.9
Bundle IDcom.automattic.alpha.woocommerce
Commit04fab3b
Installation URL67s01qbqcth3o
 Automatticians: You can use our internal self-serve MC tool to give yourself access to those builds if needed.

@iamgabrielma iamgabrielma added type: enhancement A request for an enhancement. feature: order creation All tasks related to creating an order labels Jul 30, 2025
@iamgabrielma iamgabrielma added this to the 23.0 milestone Jul 30, 2025
@iamgabrielma iamgabrielma marked this pull request as ready for review July 30, 2025 08:35
@iamgabrielma iamgabrielma requested a review from jaclync July 30, 2025 09:01
jaclync
jaclync reviewed Jul 30, 2025
View reviewed changes
Copy link
Contributor

@jaclync jaclync left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

During testing, I was still able to add a variation from a variable subscription product. Is it expected? I thought tapping on the variable subscription product would be disabled since it's grayed out.

Simulator.Screen.Recording.-.iPad.Air.13-inch.M3.-.2025-07-31.at.06.19.01.mp4

@iamgabrielma
Handle variableSubscription case from view
debe2a9 
The call to .onTapGesture for variations in the product selector was bypassing the product type check, and navigating the merchant to the variations view, where they could add subscription products to the order
@iamgabrielma
simplify, and unit tests
5a17b10
@iamgabrielma
Copy link
Contributor Author

iamgabrielma commented Jul 31, 2025

During testing, I was still able to add a variation from a variable subscription product. Is it expected? I thought tapping on the variable subscription product would be disabled since it's grayed out.

Thanks for testing, it seems my own store trolled me and variable subscriptions weren't actually variable but simple 🤦

Screenshot 2025-07-31 at 11 21 58

The problem relied on variations being handled via a call inside .tapGesture in ProductSelectorView, which was navigating the merchant to the variations view directly, bypassing the block we set for the parent product when there are no variations.

I've added a new isSubscription check on debe2a9 which now resolves the problem, as well as additional unit tests:

Screen.Recording.2025-07-31.at.11.42.28.mov

@iamgabrielma iamgabrielma requested a review from jaclync July 31, 2025 05:18
jaclync
jaclync reviewed Jul 31, 2025
View reviewed changes
Copy link
Contributor

@jaclync jaclync left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Some thoughts about the latest update, let me know what you think!

WooCommerce/Classes/ViewRelated/Products/ProductSelector/ProductSelectorView.swift Outdated
@ViewBuilder private func createProductRow(rowViewModel: ProductRowViewModel) -> some View {
if viewModel.isVariableProduct(productOrVariationID: rowViewModel.productOrVariationID),
configuration.treatsAllProductsAsSimple == false {
let isSubscription = viewModel.isSubscriptionProduct(productOrVariationID: rowViewModel.productOrVariationID)
Copy link
Contributor

@jaclync jaclync Jul 31, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

❓ Is it possible to use ProductRowViewModel.SelectedState == unsupported to determine whether a row is selectable/tappable?

Copy link
Contributor

@jaclync jaclync Jul 31, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: the DisclosureIndicator() can be hidden too if the row isn't tappable

Simulator Screenshot - iPad Air 13-inch (M3) - 2025-07-31 at 16 04 45

Copy link
Contributor Author

@iamgabrielma iamgabrielma Aug 1, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is it possible to use ProductRowViewModel.SelectedState == unsupported to determine whether a row is selectable/tappable?

That's a good point, I didn't see it right away but the existing selectionEnabled returns false when selectedState == .unsupported, which covers all unsupported product types and we won't need to perform check per-product-type, just expand the list, which is more maintainable in the long term. Updated here: b419d94 . This also comes with the subscription check being used only on tests, so I removed these here 866c91c

nit: the DisclosureIndicator() can be hidden too if the row isn't tappable

Good UI suggestion, updated: b5e48c7 so it's hidden now for non-selectable variable products.

Simulator Screenshot - iPad mini (A17 Pro) - US store - 2025-08-01 at 09 55 21

@iamgabrielma iamgabrielma requested a review from jaclync August 1, 2025 03:04
jaclync
jaclync approved these changes Aug 4, 2025
View reviewed changes
Copy link
Contributor

@jaclync jaclync left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM! :shipit:

@iamgabrielma iamgabrielma merged commit 737681a into trunk Aug 4, 2025
13 checks passed
@iamgabrielma iamgabrielma deleted the task/WOOMOB-938-prevent-subs-products-from-order-flow branch August 4, 2025 09:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@jaclync jaclync jaclync approved these changes

Assignees

No one assigned

Labels

feature: order creation All tasks related to creating an order type: enhancement A request for an enhancement.

Projects

None yet

Milestone

23.0

Development

Successfully merging this pull request may close these issues.

4 participants

@iamgabrielma @wpmobilebot @jaclync